JAMES-4231 Add S3 and file blob range reads by HesandaLiyanage · Pull Request #3244 · apache/james-project

HesandaLiyanage · 2026-10-10T11:23:20Z

Adds inclusive byte-range reads to BlobStoreDAO, with optimized implementations for S3 and file storage. S3 sends a GetObject request with a Range header. File storage seeks directly using FileChannel and allocates only the requested range, instead of loading the complete file.

start must be non-negative. Optional<Long> end specifies the inclusive last byte, or Optional.empty() to read through the end. Invalid ranges fail explicitly. Results preserve blob metadata and expose the complete source size, replacing any stale total-object-size metadata.

The default implementation reads and slices readBytes for backends without an optimized override. The S3 implementation retains the existing SSE-C request builder and namespace fallback behavior. The file implementation closes its channel, schedules blocking work on boundedElastic, and reports unexpected EOF rather than returning incomplete data.

Validation

The shared 18-case range-read contract covers Memory, File, Cassandra, PostgreSQL, S3 (Scality CloudServer), AES, and Zstd over Memory/File/S3. Zstd tests additionally force compression and check decoded ranges.

For the file optimization, ran the full FileBlobStoreDAOTest suite and Zstd-over-File range tests. Added a sparse-file regression reading four bytes at an offset beyond 2 GiB, verifying the bytes and total size without loading the whole file.

Both test suites passed locally, along with Maven Checkstyle.

mvn -o -q -pl server/blob/blob-file,server/blob/blob-zstd -am '-Dtest=FileBlobStoreDAOTest,ZstdFileBlobStoreDAOTest#readRange*' -Dsurefire.failIfNoSpecifiedTests=false test

Retriggers the PR build after an OpenSearch testcontainer infrastructure failure unrelated to the change set. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>

HesandaLiyanage · 2026-10-15T10:04:14Z

Hi @chibenwa
I’ve rebased this PR onto the new feature/s3-object-compaction branch and updated its target. It’s ready for review; all 34 focused range-read tests passed locally.
The second sub-PR, #3245 (ChunkMarker and BloomFilter GC exclusion), also targets the feature branch. Once these first two PRs are merged, I’ll build the remaining work on top of the updated feature branch following the agreed breakdown.

Arsnael

You add read-range in the BlobStoreDAO interface and I see tests for memory and s3, how is this impacting for other blobstore implementations?

HesandaLiyanage · 2026-10-16T04:08:17Z

You add read-range in the BlobStoreDAO interface and I see tests for memory and s3, how is this impacting for other blobstore implementations?

readRange is a default method, so existing implementations inherit it without needing changes. It reads the complete blob through their existing readBytes method, slices the requested range, and preserves the metadata.
This means other implementations support range reads functionally, but still incur the cost of a full-object read. S3 overrides it to request only the required bytes from storage.
The memory tests exercise the default implementation, while the S3 tests exercise its override. Backend-specific coverage for the other implementations is not included yet.

Arsnael · 2026-10-16T10:15:06Z

readRange is a default method, so existing implementations inherit it without needing changes. It reads the complete blob through their existing readBytes method, slices the requested range, and preserves the metadata.
This means other implementations support range reads functionally, but still incur the cost of a full-object read. S3 overrides it to request only the required bytes from storage.
The memory tests exercise the default implementation, while the S3 tests exercise its override. Backend-specific coverage for the other implementations is not included yet.

shouldnt this be tested at least on other implems just to make sure that it's not creating issues then?

HesandaLiyanage · 2026-10-16T10:23:14Z

shouldnt this be tested at least on other implems just to make sure that it's not creating issues then?

Yes, you’re right, I should have covered the other implementations too.

Added the contract to File, Cassandra, PostgreSQL, AES, and Zstd tests.

chibenwa · 2026-10-17T07:05:18Z

+
+    private static void validateRange(long start, long end) {
+        if (start < 0) {
+            Preconditions.checkArgument(start != Long.MIN_VALUE, "'start' must be greater than %s", Long.MIN_VALUE);


What does this Long.MIN_VALUE magic value represent ?

I'd keep start >= 0.

I used negative start values for suffix reads, with this guard rejecting an unrepresentable suffix length. I agree that this overload makes the API less clear. I’ll keep start >= 0 and handle suffix reads explicitly when adding chunk-footer support.

chibenwa · 2026-10-17T07:06:14Z

+    private static void validateRange(long start, long end) {
+        if (start < 0) {
+            Preconditions.checkArgument(start != Long.MIN_VALUE, "'start' must be greater than %s", Long.MIN_VALUE);
+            Preconditions.checkArgument(end == -1, "'end' must be -1 for a suffix range");


Let's favour explicit typing in our API

If there is no end let's represent this by Optional<Long> and rely on Optional.empty()

Agreed. Replaced the -1 sentinel with Optional; Optional.empty() now means read through the end. Updated all callers and tests.

chibenwa · 2026-10-17T07:08:04Z

            return withMetadata(ContentEncoding.NAME, contentEncoding.asValue());
        }
+
+        public BlobMetadata withTotalObjectSize(long totalObjectSize) {


I do not really understand why this is needed.

Don't you mind giving retionals as per why we'd need this total object size ?

A range response contains only the requested bytes, so its payload length doesn’t tell us the full object size. The compaction code needs that size to locate the footer and validate slot bounds without downloading the whole chunk.
S3 provides it through Content-Range; the default implementation gets it from the full read. This helper attaches that computed size, replacing any stale value already in the metadata.

Ok thanks for the answer.
Can you confirm me that this metadata is only set through range reads and never store? (That is what I understand reading the code...

It’s added only to range-read results; reading doesn’t modify the stored object. However, save() preserves supplied metadata, so explicitly saving that result would persist it. There’s currently no filter preventing that.

chibenwa

Maybe we can implement an optimized version for file storage too, while we are at it. No?

Good work overall BTW. It's way easier to digest in small chuncks.

HesandaLiyanage · 2026-10-18T05:04:19Z

Maybe we can implement an optimized version for file storage too, while we are at it. No?

Good work overall BTW. It's way easier to digest in small chuncks.

Good idea, added the optimized file implementation to this PR. It now seeks directly to the requested offset and reads only the required bytes. Tests pass locally, including a range beyond 2 GiB.

chibenwa · 2026-10-18T17:20:04Z

+            "'end' (%s) must be greater than or equal to 'start' (%s)", value, start));
+    }
+
+    private static int rangeStart(long start, long totalSize) {


Why using int as a type ? Why not long?

start and totalSize remain long. This helper returns int because the default implementation slices a Java byte array, whose indexes are int. The bounds check ensures the cast is safe. Optimized implementations such as file storage keep offsets as long, including beyond 2 GiB.

chibenwa · 2026-10-18T17:21:25Z

+    @Test
+    void readRangeShouldSeekBeyondTwoGigabytesWithoutReadingWholeFile() throws Exception {


Why isnnt this valuable test part of the contract ?

This test uses a sparse file larger than 2 GiB to verify that the optimized implementation seeks directly. Moving it into the shared contract would require a backend-specific large-object fixture. Backends using the full-read fallback also have byte-array limits, so they couldn’t pass this case. The common range behavior remains covered by the contract.

chibenwa

Really good status to me

Mostly nitpicks.

validateRange is duplicated three times (API, File, S3). It could live in a shared static helper.

Thanks for the serious work here.

Arsnael

Thank you for the follow up on the comments.

I read it and have nothing else to add. Almost there :)

HesandaLiyanage · 2026-10-19T06:26:20Z

Really good status to me

Mostly nitpicks.

validateRange is duplicated three times (API, File, S3). It could live in a shared static helper.

Thanks for the serious work here.

Thanks. Extracted validation into a shared static helper in BlobStoreDAO, reused by the default, File, and S3 implementations.

HesandaLiyanage force-pushed the JAMES-4231-range-reads branch from 538aa64 to 7d01999 Compare October 11, 2026 06:29

HesandaLiyanage and others added 4 commits October 15, 2026 15:28

JAMES-4231 Add S3 blob range reads

e04b3e1

JAMES-4231 Fix S3 range error mapping

045ea5c

JAMES-4231 Empty commit to retrigger CI

cdb749e

Retriggers the PR build after an OpenSearch testcontainer infrastructure failure unrelated to the change set. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>

JAMES-4231 Replace range size metadata and cover range boundaries

1449386

HesandaLiyanage force-pushed the JAMES-4231-range-reads branch from a38ff75 to 1449386 Compare October 15, 2026 09:59

HesandaLiyanage changed the base branch from master to feature/s3-object-compaction October 15, 2026 09:59

Arsnael reviewed Oct 16, 2026

View reviewed changes

Comment thread server/blob/blob-api/src/main/java/org/apache/james/blob/api/BlobStoreDAO.java Resolved

Comment thread ...ob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAORangeReadTest.java Resolved

JAMES-4231 Reuse S3 DAO test for range read contract

8ea019b

JAMES-4231 Cover range reads across blob store implementations

cefb9a7

chibenwa reviewed Oct 17, 2026

View reviewed changes

HesandaLiyanage added 3 commits October 18, 2026 09:46

JAMES-4231 Require non-negative range start offsets

906eeff

JAMES-4231 Represent optional range end explicitly

ba0e480

JAMES-4231 Optimize file blob range reads with positioned IO

1f1f60a

HesandaLiyanage changed the title ~~JAMES-4231 Add S3 blob range reads~~ JAMES-4231 Add S3 and file blob range reads Oct 18, 2026

chibenwa reviewed Oct 18, 2026

View reviewed changes

Arsnael reviewed Oct 19, 2026

View reviewed changes

JAMES-4231 Share blob range validation across implementations

4855e34

chibenwa approved these changes Oct 19, 2026

View reviewed changes

Arsnael approved these changes Oct 19, 2026

View reviewed changes

Arsnael merged commit 25aff41 into apache:feature/s3-object-compaction Oct 19, 2026

HesandaLiyanage mentioned this pull request Oct 20, 2026

JAMES-4231 BlobIdUpdater SPI and Cassandra reference relocation #3260

Open

		@Test
		void readRangeShouldSeekBeyondTwoGigabytesWithoutReadingWholeFile() throws Exception {

Conversation

HesandaLiyanage commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Validation

Uh oh!

HesandaLiyanage commented Oct 15, 2026

Uh oh!

Arsnael left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

HesandaLiyanage commented Oct 16, 2026

Uh oh!

Arsnael commented Oct 16, 2026

Uh oh!

HesandaLiyanage commented Oct 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chibenwa left a comment

Choose a reason for hiding this comment

Uh oh!

HesandaLiyanage commented Oct 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chibenwa left a comment

Choose a reason for hiding this comment

Uh oh!

Arsnael left a comment

Choose a reason for hiding this comment

Uh oh!

HesandaLiyanage commented Oct 19, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HesandaLiyanage commented Oct 10, 2026 •

edited

Loading